feat: add auto-cleanup for task history #8173
Conversation
- Add configuration settings for auto-cleanup (enabled, maxCount, maxDiskSpaceMB, maxAgeDays) - Create TaskHistoryCleanupService to handle cleanup logic - Implement cleanup by count, disk space, and age thresholds - Integrate cleanup service into ClineProvider with idle-time execution - Add comprehensive tests for cleanup service - Cleanup runs automatically when thresholds are exceeded - Protects tasks from current workspace from deletion - Default settings: disabled, 100 tasks max, 1GB disk space, 7 days age Fixes #8172
dosubot
bot
added
size:XL
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| for (const task of tasksToDelete) { | ||
| try { | ||
| // Calculate size before deletion for reporting | ||
| if (result.freedSpaceMB === 0) { |
There was a problem hiding this comment.
Consider always accumulating the freed space for each deleted task instead of conditionally adding it only when freedSpaceMB is zero. This ensures the reported freedSpaceMB reflects the total space freed from all deletion criteria (age, count, disk space), not just the first task.
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // Check if cleanup should be triggered | ||
| if (this.taskHistoryCleanupService.shouldTriggerCleanup(taskHistory ?? [], config)) { | ||
| // Run cleanup in the background during idle time | ||
| setTimeout(async () => { |
There was a problem hiding this comment.
Is this intentional? Using a 5-second setTimeout could cause race conditions if multiple tasks complete in quick succession. Each task update would schedule its own cleanup, potentially leading to multiple concurrent cleanup operations.
Consider using a debounced queue or mutex pattern to ensure only one cleanup runs at a time.
| for (const task of tasksToDelete) { | ||
| try { | ||
| // Calculate size before deletion for reporting | ||
| if (result.freedSpaceMB === 0) { |
There was a problem hiding this comment.
There's a potential issue here - if calculating the task size fails, freedSpaceMB remains 0, which gives incorrect metrics. We should calculate the size for all tasks to be deleted upfront.
Could we move the size calculation outside the try-catch or ensure we always calculate it?
| * Calculates the total disk space used by all tasks in MB | ||
| */ | ||
| private async calculateTotalTaskSize(taskHistory: HistoryItem[]): Promise<number> { | ||
| let totalSizeMB = 0 |
There was a problem hiding this comment.
Calculating total disk space by iterating through all tasks is expensive, especially since we're already checking disk space during the cleanup process. Could we consider:
- Only calculating when maxDiskSpaceMB is configured
- Implementing a cache that updates after each task operation
- Using filesystem metadata if available
This would significantly improve performance for users with large task histories.
| openRouterImageGenerationSelectedModel, | ||
| openRouterUseMiddleOutTransform, | ||
| featureRoomoteControlEnabled, | ||
| taskHistoryAutoCleanupEnabled: taskHistoryAutoCleanupEnabled ?? false, |
There was a problem hiding this comment.
The default values here (100 tasks, 1000MB, 7 days) might be too aggressive for some users. Have we considered:
- More conservative defaults (e.g., 200 tasks, 2000MB, 14 days)?
- Making these workspace-specific rather than global?
- Adding a first-run prompt to let users configure their preferences?
This could prevent accidental data loss for users who don't realize cleanup is happening.
| vi.useRealTimers() | ||
| }) | ||
|
|
||
| describe("performCleanup", () => { |
There was a problem hiding this comment.
We're missing test coverage for:
- Concurrent cleanup attempts (multiple tasks finishing simultaneously)
- Edge case where a task has no timestamp
- Cleanup failure recovery (partial cleanup success)
- Tasks with corrupted/missing directories
Could we add these scenarios to ensure robustness?
- Add safety check for globalStorageUri before initializing TaskHistoryCleanupService - Prevents test failures when globalStorageUri is not mocked in test context
|
This PR creates new settings that are not exposed in the UI and I think this should be configurable. |
This PR attempts to address Issue #8172 by implementing an auto-cleanup feature for task history.
Summary
Implements automatic cleanup of task history to prevent disk space from expanding rapidly when checkpoints are enabled. The feature is disabled by default to maintain backward compatibility.
Changes
TaskHistoryCleanupServiceto handle cleanup logicClineProviderwith idle-time executionConfiguration
The feature can be configured through VSCode settings:
taskHistoryAutoCleanupEnabled: Enable/disable auto-cleanup (default: false)taskHistoryMaxCount: Maximum number of tasks to keep (default: 100)taskHistoryMaxDiskSpaceMB: Maximum disk space in MB (default: 1024)taskHistoryMaxAgeDays: Maximum age in days (default: 7)Testing
All tests pass including:
Fixes #8172
Feedback and guidance are welcome!
Important
Introduces auto-cleanup for task history in
ClineProviderusingTaskHistoryCleanupService, configurable via VSCode settings, with comprehensive tests.ClineProviderandTaskHistoryCleanupService.taskHistoryAutoCleanupEnabled,taskHistoryMaxCount,taskHistoryMaxDiskSpaceMB,taskHistoryMaxAgeDays.TaskHistoryCleanupServicefor cleanup logic.ClineProviderwith idle-time execution.global-settings.tsto include new configuration options.TaskHistoryCleanupServiceinTaskHistoryCleanupService.spec.ts.This description was created by
for d4a370e. You can customize this summary. It will automatically update as commits are pushed.